Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SQL: Fix issue with complex expression as args of PERCENTILE/_RANK #37102

Merged
merged 2 commits into from
Jan 24, 2019

Conversation

matriv
Copy link
Contributor

@matriv matriv commented Jan 3, 2019

When the arguments of PERCENTILE and PERCENTILE_RANK can be folded,
the ConstantFolding rule kicks in and calls the replaceChildren()
method on InnerAggregate which is created from the aggregation rules
of the Optimizer. InnerAggregate in turn, cannot implement the method
as the logic of creating a new InnerAggregate instance from a list of
Expressions resides in the Optimizer. So, instead, ConstantFolding
should be applied before any of the aggregations related rules.

Fixes: #37099

When the arguements of PERCENTILE and PERCENTILE_RANK can be folded,
the `ConstantFolding` rule kicks in and calls the `replaceChildren()`
method on `InnerAggregate` which is created from the aggregation rules
of the `Optimizerz. `InnerAggregate` in turn, cannot implement the method
as the logic of creating a new `InnerAggregate` instance from a list of
`Expression`s resides in the Optimizer. So, instead, `ConstantFolding`
should be applied before any of the aggregations related rules.

Fixes: elastic#37099
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search

@matriv matriv requested review from costin and astefan January 3, 2019 10:41
@matriv
Copy link
Contributor Author

matriv commented Jan 3, 2019

@costin maybe you have a better suggestion for this

@matriv matriv added v6.6.0 and removed v6.6.1 labels Jan 4, 2019
@pcsanwald pcsanwald removed the v6.6.0 label Jan 17, 2019
Copy link
Member

@costin costin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@matriv matriv added the v6.6.1 label Jan 24, 2019
Copy link
Contributor

@astefan astefan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@matriv matriv merged commit 74b6f30 into elastic:master Jan 24, 2019
@matriv matriv deleted the mt/fix-37099 branch January 24, 2019 16:40
matriv added a commit that referenced this pull request Jan 24, 2019
…37102)

When the arguements of PERCENTILE and PERCENTILE_RANK can be folded,
the `ConstantFolding` rule kicks in and calls the `replaceChildren()`
method on `InnerAggregate` which is created from the aggregation rules
of the `Optimizerz. `InnerAggregate` in turn, cannot implement the method
as the logic of creating a new `InnerAggregate` instance from a list of
`Expression`s resides in the Optimizer. So, instead, `ConstantFolding`
should be applied before any of the aggregations related rules.

Fixes: #37099
matriv added a commit that referenced this pull request Jan 24, 2019
…37102)

When the arguements of PERCENTILE and PERCENTILE_RANK can be folded,
the `ConstantFolding` rule kicks in and calls the `replaceChildren()`
method on `InnerAggregate` which is created from the aggregation rules
of the `Optimizerz. `InnerAggregate` in turn, cannot implement the method
as the logic of creating a new `InnerAggregate` instance from a list of
`Expression`s resides in the Optimizer. So, instead, `ConstantFolding`
should be applied before any of the aggregations related rules.

Fixes: #37099
matriv added a commit that referenced this pull request Jan 24, 2019
…37102)

When the arguements of PERCENTILE and PERCENTILE_RANK can be folded,
the `ConstantFolding` rule kicks in and calls the `replaceChildren()`
method on `InnerAggregate` which is created from the aggregation rules
of the `Optimizerz. `InnerAggregate` in turn, cannot implement the method
as the logic of creating a new `InnerAggregate` instance from a list of
`Expression`s resides in the Optimizer. So, instead, `ConstantFolding`
should be applied before any of the aggregations related rules.

Fixes: #37099
@matriv
Copy link
Contributor Author

matriv commented Jan 24, 2019

Backported to 6.x with 4a1e66e
to 6.6 with 0edaf2b
to 6.5 with e48982c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants